-
Notifications
You must be signed in to change notification settings - Fork 14.1k
CTFE: avoid emitting a hard error on generic normalization failures #149501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
😢 |
|
@rust-lang/types would like to beta backport this as early as possible ideally. Can I get one of you to quickly look at this PR? |
In the long term I think this is the right fix 🤔 Before this PR were we getting similar kinds of ICEs due to opaque types? I was looking at the doc comments for I'm not sure I feel confident in being able to judge whether there are any weird side effects from this PR or not. I'd certainly feel better to revert #147793 and then land this "normally" or if it really will be the near future that we fix #149283 then land that and then re-land #147793 |
|
Good catch @BoxyUwU, there are apparently other "not quite a bug" reasons for normalization failures in well-formed programs. This PR causes us to also weaken CTFE errors due to param-env shadowing from a hard-error to simply returning Now, not erroring in these cases also feels desirable to me. However, this is something I did not consider when writing this PR. More thoughts on this later |
soon-ish? no it depends on whether somebody ends up working on this, it'd probably take a few months to get something working here (with mentoring) for a somewhat experienced contributor. |
|
@bors r=lqd,oli-obk |
|
💡 This pull request was already approved, no need to approve it again.
|
…, r=lqd,oli-obk CTFE: avoid emitting a hard error on generic normalization failures Fixes rust-lang#149081. The fix is quite unsatisfying and should not be necessary, cc rust-lang#149283. That change is significantly more involved. This temporary fix introduces some unnecessary complexity and may hide other type system bugs. cc `@rust-lang/types` I think we should try to fix issue rust-lang#149283 in the near future and then remove this hack again. I originally intended a more targeted fix. I wanted to skip evaluating constants in MIR opts if their body was polymorphic and the current generic arguments still reference generic parameters. Notes from looking into this: - we only fetch the MIR in the `eval_to_allocation_raw` query - figuring out which MIR to use is hard - dealing with trivial consts is annoying - need to resolve instances for associated consts - implementing this by hand is hard - inlining handles this issue by bailing on literally all normalization failures, even the ones that imply an unsoundness - `try_normalize_after_erasing_regions` generally does two things - deal with ambiguity after inlining - deal with error tainting issues (please don't, we should stop doing that) - CTFE could be changed to always silently ignore normalization failures if we're in a generic body - hides actual bugs <- went with this option r? types
Rollup of 9 pull requests Successful merges: - #147841 (Fix ICE when applying test macro to crate root) - #149501 (CTFE: avoid emitting a hard error on generic normalization failures) - #149517 (Implement blessing for tidy alphabetical check) - #149521 (Improve `io::Error::downcast`) - #149545 (fix the check for which expressions read never type) - #149549 (Regression test for system register `ttbr0_el2`) - #149579 (Motor OS: fix compile error) - #149595 (Tidying up `tests/ui/issues` tests [2/N]) - #149597 (Revert "implement and test `Iterator::{exactly_one, collect_array}`") r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #147841 (Fix ICE when applying test macro to crate root) - #149501 (CTFE: avoid emitting a hard error on generic normalization failures) - #149517 (Implement blessing for tidy alphabetical check) - #149521 (Improve `io::Error::downcast`) - #149545 (fix the check for which expressions read never type) - #149549 (Regression test for system register `ttbr0_el2`) - #149579 (Motor OS: fix compile error) - #149595 (Tidying up `tests/ui/issues` tests [2/N]) - #149597 (Revert "implement and test `Iterator::{exactly_one, collect_array}`") r? `@ghost` `@rustbot` modify labels: rollup
Fixes #149081.
The fix is quite unsatisfying and should not be necessary, cc #149283. That change is significantly more involved. This temporary fix introduces some unnecessary complexity and may hide other type system bugs.
cc @rust-lang/types I think we should try to fix issue #149283 in the near future and then remove this hack again.
I originally intended a more targeted fix. I wanted to skip evaluating constants in MIR opts if their body was polymorphic and the current generic arguments still reference generic parameters. Notes from looking into this:
eval_to_allocation_rawquerytry_normalize_after_erasing_regionsgenerally does two thingsr? types